-
Notifications
You must be signed in to change notification settings - Fork 259
Adds additional properties to {Left,Right}Module #2906
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
jamesmckinna
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice addition, but I think the suggested renaming/refactorings make it 'better'?
(Open to debate on this, though! esp. with any 2nd reviewer...)
using suggestion Co-authored-by: jamesmckinna <[email protected]>
jamesmckinna
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great now! Thanks for the prompt response to my feedback.
|
This is very good! But I'd like to see the corresponding proofs on right modules as well in this PR |
|
I agree with @Taneb regarding the |
Yep. I'll add these soonish. |
|
Ok -- added RightModule properties. |
Co-authored-by: jamesmckinna <[email protected]>
Co-authored-by: jamesmckinna <[email protected]>
Co-authored-by: jamesmckinna <[email protected]>
Co-authored-by: jamesmckinna <[email protected]>
Co-authored-by: jamesmckinna <[email protected]>
|
Nice! All looks great now. Will merge in the next 24 hours unless we hear objections! |
This PR adds additional properties to
Algebra.Module.Properties.LeftModule.NOTE: I can also add the corresponding properties to
RightModule, but I'll wait for any discussions of naming and appropriateness of the LeftModule properties to conclude first.